Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flap function to Apply #2588

Merged
merged 5 commits into from
Nov 16, 2018
Merged

Add flap function to Apply #2588

merged 5 commits into from
Nov 16, 2018

Conversation

ssanj
Copy link
Contributor

@ssanj ssanj commented Oct 30, 2018

Add the flap function to Apply to facilitate combining a function in an
Apply context with a value which is not.

This is similar to ap:

def ap[A, B](ff: F[A => B])(fa: F[A]): F[B]

with the minor change of the value not being in a context:

def flap[A, B](ff: F[A => B])(a: A): F[B]
                              ^^^^

Thank you for contributing to Cats!

This is a kind reminder to run sbt prePR and commit the changed files before submitting.

Add the `flap` function to `Apply` to facilitate combining a function in an
`Apply` context with a value which is not.

This is similar to `ap`:

```
def ap[A, B](ff: F[A => B])(fa: F[A]): F[B]
```

with the minor change of the value not being in a context:

```
def flap[A, B](ff: F[A => B])(a: A): F[B]
                              ^^^^
```
@LukaJCB
Copy link
Member

LukaJCB commented Oct 30, 2018

Hi @ssanj, thanks for contributing! I don't think Apply is the correct type class here as it could simply be implemented with Functor instead. Futhermore I'm not sure flap is the best name for this, but I'm also not coming up with a better name right now unfortunately, might think about this some more :)

@ssanj
Copy link
Contributor Author

ssanj commented Oct 30, 2018

You're right. This could be moved into Functor. The reason I moved it into Apply was that it had a function in a context (F[A => B]) and was verify similar to ap. Happy to move it.

Yeah, naming this is hard. I hard a look at some of the Haskell equivalents and the variants I saw were:

Another question I had was should this obey any specific Laws?

@ssanj
Copy link
Contributor Author

ssanj commented Oct 31, 2018

When I move the flap function into Functor I get the following compiler error:

[error] /Volumes/Work/projects/code/scala/opensource/cats/core/src/main/scala/cats/Functor.scala:56:21: not found: type A
[error]   def widen[A, B >: A](fa: F[A]): F[B] = fa.asInstanceOf[F[B]]
[error]                     ^
[error] /Volumes/Work/projects/code/scala/opensource/cats/core/src/main/scala/cats/Functor.scala:12:2: type arguments [C,B] do not conform to method widen's type parameter bounds [A,B >: A]
[error] @typeclass trait Functor[F[_]] extends Invariant[F] { self =>
[error]  ^
[error] two errors found
[error] /Volumes/Work/projects/code/scala/opensource/cats/core/src/main/scala/cats/Functor.scala:56:21: not found: type A
[error]   def widen[A, B >: A](fa: F[A]): F[B] = fa.asInstanceOf[F[B]]
[error]                     ^
[error] /Volumes/Work/projects/code/scala/opensource/cats/core/src/main/scala/cats/Functor.scala:12:2: type arguments [C,B] do not conform to method widen's type parameter bounds [A,B >: A]
[error] @typeclass trait Functor[F[_]] extends Invariant[F] { self =>
[error]  ^
[error] two errors found

I've enquired whether this is a Simulcrum issue. I can get this to work if I rewrite widen not to have a upper bound:

def widen[A, B](fa: F[A])(implicit ev: A <:< B): F[B] = fa.asInstanceOf[F[B]]

Has anyone seen this issue before?

@kailuowang
Copy link
Contributor

How about we call this mapApply with a F[_] :Functor bound add it to a Function1 syntax extension trait? We can't add any method to any trait without breaking binary compatibility in Scala 2.11. And we promised to not break it until 2.0

@codecov-io
Copy link

codecov-io commented Nov 7, 2018

Codecov Report

Merging #2588 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2588      +/-   ##
==========================================
+ Coverage   95.16%   95.17%   +0.01%     
==========================================
  Files         361      362       +1     
  Lines        6634     6655      +21     
  Branches      294      302       +8     
==========================================
+ Hits         6313     6334      +21     
  Misses        321      321
Impacted Files Coverage Δ
core/src/main/scala/cats/syntax/function1.scala 100% <100%> (ø)
core/src/main/scala/cats/data/Chain.scala 99.58% <0%> (+0.01%) ⬆️
core/src/main/scala/cats/data/NonEmptyChain.scala 99.07% <0%> (+0.07%) ⬆️
...ore/src/main/scala/cats/data/NonEmptyMapImpl.scala 96.15% <0%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5bd5b0...7ff5d8f. Read the comment docs.

@@ -31,6 +31,7 @@ trait AllSyntax
with EqSyntax
with FlatMapSyntax
with FoldableSyntax
with Function1Syntax
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

due to binary compatibility restrictions, you can't add it to this trait either. However, you can add it to AllSyntaxBinCompat3 which is a new trait that hasn't been released yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. 👍

@ssanj
Copy link
Contributor Author

ssanj commented Nov 15, 2018

@kailuowang @LukaJCB does this look ok now?

@kailuowang
Copy link
Contributor

kailuowang commented Nov 15, 2018

I am going to schedule this to 1.5 so that you don't have to change your BinaryCompat trait.

@kailuowang kailuowang changed the base branch from master to 1.5.x November 15, 2018 16:17
@kailuowang kailuowang requested a review from LukaJCB November 15, 2018 20:32
@kailuowang kailuowang changed the base branch from 1.5.x to master November 16, 2018 02:16
@kailuowang kailuowang merged commit 32e2aa6 into typelevel:master Nov 16, 2018
@kailuowang kailuowang added this to the 1.5.0-RC1 milestone Nov 16, 2018
@ssanj ssanj deleted the flap branch March 30, 2019 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants